-
-
Notifications
You must be signed in to change notification settings - Fork 181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow Enabling 90Hz mode for Oculus #108
base: main
Are you sure you want to change the base?
Conversation
// VrApi : FPS=90/90,Prd=33ms,Tear=0,Early=0... | ||
float targetDisplayRefresh = App.UserConfig.Video.DisplayRefresh; | ||
float[] freqs = OVRManager.display.displayFrequenciesAvailable; | ||
foreach (float freq in freqs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't sure if we should do this check here or in UserOptions, but it needs the OVRManager
and that was throwing exceptions in UserOptions, so I left it here
Assets/Scripts/VrSdk.cs
Outdated
m_VrCamera.gameObject.AddComponent<OculusPreCullHook>(); | ||
|
||
gameObject.AddComponent<OculusMRCCameraUpdate>(); | ||
// ---------------------------------------------------------------------------------------- // |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reformatting here was done manually, and wasn't caught in #81 because of dotnet/format#1101 (and probably the same basic issue in Rider)
@@ -425,6 +426,17 @@ public float OdsPoleCollapsing | |||
} | |||
} | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't sure if Video
was the right section for this, but I didn't see anything better. It seems like it's mostly video out stuff...
updated to tip |
Can we make this draft? |
Done, though I think it's ready for review, just not for merge. How can I indicate that? |
I think maybe we just need a discussion. My feeling is that the benefits aren't enough to warrant the hassle right now and we should maybe revisit this later. So my vote is to move it to a Github issue or discussion. |
90Hz mode offers a much smoother experience, if we can keep it up. Jumping framerates are probably worse, though, than locking at 72Hz. This PR was an attempt at making it easier to test if 90Hz is suitable for real world use in OB. I still think it serves that purpose, although if we don't have anyone with time to test and validate it, then I agree that it's not really going to go anywhere. But without the PR, I know it will never get anywhere... (so yes, I opened this to try to encourage discussion. I guess it didn't work) |
One way to mitigate the performance cost of this would be to combine it with Application Spacewarp: https://developer.oculus.com/blog/introducing-application-spacewarp/ But our extensive use of transparancy might rule that out. Anyone got any thoughts? |
Note that Spacewarp currently hard requires URP (and OpenXR, but we're working on that) to function, so it's a bigger task! It sounds like a really useful feature though. |
Before we talk about performance cost... can we try this and see? We know that our average frame rate (from Oculus metrics) is just a hair under 72. I don't know if that means we'll never hit 90 in the real world, or it's easily capable of doing target-2. |
@mikeage we've had reports that xr_v2 has gained some performance compared to the main release. is bumping to 90 by default possible? |
Sure -- do we have a set of reference files to test with? (I need to confirm how we check FPS; I don't think we have the VrApi prints anymore). But we can try enabling it on xr_v2. |
I'm mildly -1 on this. (-0.5?) A lot of our users push performance to the limit. Gaining performance when we switch to xr is a good thing. Is this the best thing to spend it on? If it's the default, how easy is it for people to switch back? How many will even think of switching back? There's a bunch of things disabled going from PC to Quest (shadows, bloom after a certain complexity) that we could think about reenabling instead of this. |
godotengine/godot#67179 (comment) Just wanting to note that the PR to Godot to add https://registry.khronos.org/OpenXR/specs/1.0/html/xrspec.html#XR_FB_display_refresh_rate suggests that it's supported on SteamVR as well! We could make a cross-platform implementation of this. |
By adding:
This PR will allow setting the default display refresh to 90Hz or, presumably, if you have v28, to 120Hz.